Skip to content

Enh/getcachednamespaces - #1432

Closed
oruebel wants to merge 12 commits into
devfrom
enh/getcachednamespaces
Closed

oruebel wants to merge 12 commits into
devfrom
enh/getcachednamespaces

Conversation

@oruebel

@oruebel oruebel commented Feb 18, 2022 •

Copy link
Copy Markdown
Contributor

Motivation

#1431

Relate to dandi/dandi-cli#917

This PR extracts the code to determine the cached extension namespace to use for validation from the main function of the command line validator to a separate function so that the code can be used outside to validate files against cached namespaces.

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Have you checked our Contributing document?
  • Have you ensured the PR clearly describes the problem and the solution?
  • Is your contribution compliant with our coding style? This can be checked running flake8 from the source directory.
  • Have you checked to ensure that there aren't other open Pull Requests for the same change?
  • Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

@oruebel
oruebel requested a review from rly February 18, 2022 20:35
@oruebel oruebel added category: enhancement improvements of code or code behavior topic: validator issues related to validation of files labels Feb 18, 2022
@oruebel

oruebel commented Feb 23, 2022

Copy link
Copy Markdown
Contributor Author

@rly this issue would be good to address in the next minor release

@codecov

codecov Bot commented Mar 17, 2022 •

Copy link
Copy Markdown

Codecov Report

Merging #1432 (f5ab53d) into dev (4460a1b) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev    #1432      +/-   ##
==========================================
- Coverage   78.42%   78.34%   -0.09%     
==========================================
  Files          37       37              
  Lines        2777     2780       +3     
  Branches      493      493              
==========================================
  Hits         2178     2178              
- Misses        518      521       +3     
  Partials       81       81              
Impacted Files Coverage Δ
src/pynwb/validate.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4460a1b...f5ab53d. Read the comment docs.

@rly

rly commented Mar 17, 2022

Copy link
Copy Markdown
Contributor

@oruebel what do you think about also extracting most of the code in __main__ in src/pynwb/validate.py to a function that takes as inputs the arguments passed into the CLI function. Then a user can basically call the CLI script using Python without using the shell.

@oruebel

oruebel commented Mar 17, 2022

Copy link
Copy Markdown
Contributor Author

what do you think about also extracting most of the code in __main__ in src/pynwb/validate.py to a function

that makes sense

@bendichter

Copy link
Copy Markdown
Collaborator

We agreed to incorporate this feature into validate

@yarikoptic

Copy link
Copy Markdown
Contributor

would this PR be merged/released some time soon?

@CodyCBakerPhD

Copy link
Copy Markdown
Collaborator

@yarikoptic I'm close to finishing something very similar that also integrates with the dandi.pynwb_utils.validate, look for that in next day or so

@jwodder jwodder left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointed out some minor coding improvements.

Comment thread src/pynwb/validate.py Outdated
Comment thread src/pynwb/validate.py Outdated
rly and others added 2 commits June 30, 2022 12:05
Co-authored-by: John T. Wodder II <jwodder@users.noreply.github.com>
Co-authored-by: John T. Wodder II <jwodder@users.noreply.github.com>
@rly

rly commented Jun 30, 2022

Copy link
Copy Markdown
Contributor

Pointed out some minor coding improvements.

Thanks, @jwodder !

@rly rly mentioned this pull request Jul 8, 2022
6 tasks
@yarikoptic

Copy link
Copy Markdown
Contributor

this one is open since July -- any plans to finalize/release it ?

@oruebel

oruebel commented Nov 1, 2022

Copy link
Copy Markdown
Contributor Author

this one is open since July -- any plans to finalize/release it ?

@yarikoptic thanks for following up. I think this one got put on hold while #1494 was being worked out and I think that PR may potentially already address that issue. I'll bring it up during our developer meeting tomorrow to see whether this PR should be merged or whether it should be closed in favor of #1494

@CodyCBakerPhD

Copy link
Copy Markdown
Collaborator

@yarikoptic This has indeed already been merged in #1511, this can be closed here and I'll get a PR up on the dandi-cli to adjust the API calls to have this functionality

@oruebel

oruebel commented Nov 1, 2022

Copy link
Copy Markdown
Contributor Author

Closing this PR since this has been addressed in #1511

@oruebel oruebel closed this Nov 1, 2022
@oruebel
oruebel deleted the enh/getcachednamespaces branch November 1, 2022 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: enhancement improvements of code or code behavior topic: validator issues related to validation of files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants